Skip to content

Remove one level of indirection from DVR classes#2750

Draft
maxnoe wants to merge 1 commit into
mainfrom
remove_indirection
Draft

Remove one level of indirection from DVR classes#2750
maxnoe wants to merge 1 commit into
mainfrom
remove_indirection

Conversation

@maxnoe
Copy link
Copy Markdown
Member

@maxnoe maxnoe commented May 12, 2025

No description provided.

@maxnoe maxnoe requested review from Hckjs and kosack May 12, 2025 09:36
@maxnoe maxnoe force-pushed the remove_indirection branch 2 times, most recently from b1761d8 to 3b235a2 Compare May 12, 2025 09:51
kosack
kosack previously approved these changes May 12, 2025
@kosack
Copy link
Copy Markdown
Member

kosack commented May 12, 2025

Looks good, just need to include the tel_id and selected_gain_channel in the tests and other places where it is used.

Hckjs
Hckjs previously approved these changes May 12, 2025
@Hckjs
Copy link
Copy Markdown
Contributor

Hckjs commented May 12, 2025

One could separate the actual reduction algorithm in the __call__-function to its own function and call it in the Reducers __call__-function to be more coherent with the ImageCleaners. That would make it also possible to call that algorithms by its own.

@maxnoe maxnoe dismissed stale reviews from Hckjs and kosack via 0d80db3 May 12, 2025 13:36
@maxnoe maxnoe force-pushed the remove_indirection branch from 3b235a2 to 0d80db3 Compare May 12, 2025 13:36
@ctao-dpps-sonarqube
Copy link
Copy Markdown

Passed

Analysis Details

0 Issues

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 0 Code Smells

Coverage and Duplications

  • Coverage 100.00% Coverage (93.70% Estimated after merge)
  • Duplications 0.00% Duplicated Code (0.70% Estimated after merge)

Project ID: cta-observatory_ctapipe_AY52EYhuvuGcMFidNyUs

View in SonarQube

@maxnoe
Copy link
Copy Markdown
Member Author

maxnoe commented May 12, 2025

One could separate the actual reduction algorithm in the call-function to its own function and call it in the Reducers call-function to be more coherent with the ImageCleaners. That would make it also possible to call that algorithms by its own.

For the image cleaners, the signature between __call__ and the lower-level functions is different. Here it is the same.

@maxnoe
Copy link
Copy Markdown
Member Author

maxnoe commented Aug 29, 2025

Converting to draft.

It seems weird to give cleaning algorithms access to the monitoring data, but not the DVR.

Let's fix that here.

@maxnoe maxnoe marked this pull request as draft August 29, 2025 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants